Conversation
ValyaB
changed the title
|
If we are approving kubelet serving certificates, we probably should match k8s recommendation - see docs that offer 3 conditions for valid serving certificates |
added validating except IP and DNS, created as a separate ticket https://castai.atlassian.net/browse/KUBE-1031 |
| const ( | ||
| approveCSRTimeout = 4 * time.Minute | ||
| ) | ||
| // TODO deprecated action |
There was a problem hiding this comment.
Why not remove the code entirely?
There was a problem hiding this comment.
The EP side can send it. I want to ensure it gets approved if EP sends it.
There was a problem hiding this comment.
Why would EP send it? Even if it sends it, we will do nothing for the action itself.
if we want to have a "deprecated actions Noop" handler, then let's name it this way so it's usable in other situations.
But this is still dead code imo and can be handled by the "missing handler" path if it ever receives the action by chance.
There was a problem hiding this comment.
renamed to ApproveCSRHandlerDeprecated
There was a problem hiding this comment.
We should process and acknowledge the action to maintain logical consistency.
There was a problem hiding this comment.
If EP sends approve csr action, we should handle and acknowledge it.
if we not acknowledged - CH will try to send it again or fail AddNode action
internal/actions/csr/csr.go
Outdated
|
|
||
| func (c *Certificate) validateCSR(csr *x509.CertificateRequest) error { | ||
| if c.SignerName == certv1.KubeletServingSignerName { | ||
| if len(csr.Subject.CommonName) == 0 { |
There was a problem hiding this comment.
Common name must also match the requester (the node name), right?
internal/actions/csr/csr.go
Outdated
| } | ||
|
|
||
| func (c *Certificate) SignerName() string { | ||
| // node-csr prefix for bootstrap kubelet csr. |
2 participants
No description provided.